-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
425 more logging only first trackable added comes online when stationary #518
Closed
lawrence-forooghian
wants to merge
8
commits into
main
from
425-MORE-LOGGING-only-first-trackable-added-comes-online-when-stationary
Closed
425 more logging only first trackable added comes online when stationary #518
lawrence-forooghian
wants to merge
8
commits into
main
from
425-MORE-LOGGING-only-first-trackable-added-comes-online-when-stationary
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I want it to be easy for people to create a script that uses the LogParser library. The easiest thing I can think of is to be able to just copy and paste the existing example app and then make modifications to it. This isn’t very easy with the current structure, since the example app’s code and build config is a bit intertwined with that of the LogParser library. By putting the example app in a separate directory, we can make a copy of it by just duplicating the entire directory.
I intend to add other things at the top level of the Tools directory (like tools that actually make use of the LogParser library).
While testing a fix for #425, I wanted to have access to the location history data in a log message. This adds a switch in a new "Developer settings" section of the Settings page, which when turned on will emit a JSON serialization of the location history data when it’s received from the SDK. It’s turned off by default, because it's only for my specific use case.
…ample app logs This removes this caveat from ExampleAppSDKLogLine#message: > The ``SDKLogMessage/message`` of this value is not necessarily the exact > value that was passed as the `message` argument of > AblyAssetTrackingCore’s `LogHandler.logMessage(level:message:error:)`. > It may contain a suffix added by the example apps’ log handlers, to add > a trailing full stop and information about the `error` argument. Now, #message contains the exact value passed as the `message` argument to `LogHandler.logMessage(level:message:error:)`, and #errorDescription contains the localizedDescription of the error passed as the `error` argument to that method. I want access to the exact log message emitted by the SDK, without any additional characters, so that I can for example emit some JSON in a log message from the SDK and then parse this JSON in an app that reads the logs.
I noticed that on a stationary device, or on a simulator set to simulate a static location, the first trackable added would come online almost immediately, but subsequent ones would not come online or sometimes would come online after a long delay. This is because a trackable will not come online until, after the trackable is added, we receive a location update from LocationService. The frequency of location updates from PassiveLocationManager is determined by that of CLLocationManager, which usually only emits location updates when the device has moved a sufficient distance (where “sufficient” is determined by the publisher’s resolution policy). So, we provide a mechanism for the publisher to request a location update from CLLocationManager independently of the device’s motion, and we request a location update when any trackable (except for the first) is added. It’s probably worth also explaining how Android behaves in this scenario and why. My notes here are based on the codebase at commit 3a0d833. There are two different things that happen in the Android asset tracking SDK, either of which has the side effect of causing the publisher to receive a location update when a new trackable is added: 1. The publisher calls DefaultMapbox.changeResolution, which in turn ends up removing and re-adding all of (Google SDK) FusedLocationProviderClient’s location observers (via its requestLocationUpdates method). The documentation for FusedLocationProviderClient.requestLocationUpdates implies that observers will always receive a location update after this method is called (although it might be an old location). 2. (Only in the case where the added trackable is set as the active trackable – i.e. when it is added using .track and not .add) The publisher calls DefaultMapbox.clearRoute, which calls (Mapbox SDK) MapboxNavigation.setNavigationRoutes. Through some process that I haven’t looked into, this ends up calling (Mapbox SDK) MapboxTripSession.kt’s navigatorObserver’s onStatus, which makes an explicit call to updateLocationMatcherResult, which emits a location update. I think that the solution I’ve implemented for iOS turns Android side effect 1 into an explicit behaviour. Closes #425.
I need this logging to help me analyse the impact of the change in a73e823.
This is intended to help me analyse the impact of the change in a73e823.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will be pushed to #481 once #515, #516 and #517 are merged.